refactor(config/test): add mustUnmarshalTOML helper and consolidate temp-file patterns#5547
Merged
Merged
Conversation
- Add mustUnmarshalTOML helper using toml.Unmarshal to config_core_test.go for tests that need a populated struct without the full LoadFromFile validation pipeline (implements BurntSushi/toml Go Fan recommendation) - Refactor config_test.go: replace 16 verbose t.TempDir()+filepath.Join+ os.WriteFile patterns with writeTempTOML helper calls - Refactor config_guardpolicies_test.go: replace 4 verbose patterns with writeTempTOML helper calls - Remove unused path/filepath imports from config_test.go and config_guardpolicies_test.go
Copilot
AI
changed the title
[WIP] Review Go module BurntSushi/toml usage in gh-aw-mcpg
refactor(config/test): add mustUnmarshalTOML helper and consolidate temp-file patterns
May 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors internal/config Go tests to reduce repetitive temp-file boilerplate when testing LoadFromFile, and introduces a helper for tests that only need TOML unmarshalling without running the full file/validation pipeline.
Changes:
- Consolidate repeated
t.TempDir+filepath.Join+os.WriteFiletest patterns by usingwriteTempTOML. - Add
mustUnmarshalTOMLhelper for directtoml.Unmarshalin unit tests that don’t needLoadFromFile. - Remove now-unused
path/filepathimports from updated test files.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config_test.go | Replaces many inline temp-file creation sequences with writeTempTOML for TOML-based LoadFromFile tests. |
| internal/config/config_guardpolicies_test.go | Updates TOML guard policy tests to use writeTempTOML, reducing duplicated temp-file setup. |
| internal/config/config_core_test.go | Adds mustUnmarshalTOML helper alongside existing writeTempTOML to support non-file-based TOML parsing in tests. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
Comment on lines
983
to
986
| func TestLoadFromFile_InvalidTOMLWithLineNumber(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
| tmpFile := filepath.Join(tmpDir, "config.toml") | ||
|
|
||
| // Invalid TOML: unterminated string on line 2 | ||
| tomlContent := `[servers.test] | ||
| command = "docker | ||
| args = ["run"] | ||
| ` | ||
| path := writeTempTOML(t, "[servers.test]\ncommand = \"docker\nargs = [\"run\"]\n") | ||
|
|
Collaborator
|
@copilot merge main, address review feedback, and fix linting ci errors |
Contributor
Author
Done. I merged main, addressed the review feedback, and fixed the lint CI failure:
All config tests pass and |
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ Enhancement
The Go Fan review of
BurntSushi/tomlflagged one actionable test ergonomics improvement: usetoml.Unmarshaldirectly in tests that don't need the fullLoadFromFilepipeline, and consolidate the scattered verbose temp-file boilerplate.What does this improve?
config_test.goandconfig_guardpolicies_test.gowere each manually constructingt.TempDir() + filepath.Join + os.WriteFile— identical 4-line sequences thatwriteTempTOMLalready exists to replace.Implementation approach:
mustUnmarshalTOMLadded toconfig_core_test.go— thin wrapper aroundtoml.Unmarshalfor unit tests operating on already-parsed structs, avoiding temp files entirely:config_test.goand 4 inconfig_guardpolicies_test.gowithwriteTempTOMLcalls."path/filepath"import from both files.No production code changed.